fix(readonly): apply ACL also to files inside read-only folders

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp
index 3150819a33a3eb88591cba55dd709cddb3b1bc49..c2a7b8ed508c38d1efb3f67b6ccfe23c33bfeae8 100644 (file)
--- a/src/libsync/filesystem.cpp
+++ b/src/libsync/filesystem.cpp
@@ -416,7 +416,9 @@ bool FileSystem::setFolderPermissions(const QString &path,
 
     if (permissions == FileSystem::FolderPermissions::ReadOnly) {
         qCInfo(lcFileSystem) << path << "will be read only";
-        if (!AddAccessDeniedAce(newDacl.get(), ACL_REVISION, FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA | FILE_DELETE_CHILD, sid)) {
+
+        if (!AddAccessDeniedAceEx(newDacl.get(), ACL_REVISION, OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE,
+                                  FILE_DELETE_CHILD | DELETE | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA, sid)) {
             qCWarning(lcFileSystem) << "error when calling AddAccessDeniedAce << path" << GetLastError();
             return false;
         }
@@ -468,8 +470,18 @@ bool FileSystem::setFolderPermissions(const QString &path,
         return false;
     }
 
-    if (!SetFileSecurityW(path.toStdWString().c_str(), info, &newSecurityDescriptor)) {
-        qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << path << GetLastError();
+    auto currentFolder = QDir{path};
+    const auto childFiles = currentFolder.entryList(QDir::Filter::Files);
+    for (const auto &oneEntry : childFiles) {
+        const auto childFile = QDir::toNativeSeparators(path + QDir::separator() + oneEntry);
+        if (!SetFileSecurityW(childFile.toStdWString().c_str(), info, &newSecurityDescriptor)) {
+            qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << childFile << GetLastError();
+            return false;
+        }
+    }
+
+    if (!SetFileSecurityW(QDir::toNativeSeparators(path).toStdWString().c_str(), info, &newSecurityDescriptor)) {
+        qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << QDir::toNativeSeparators(path) << GetLastError();
         return false;
     }
 #else

diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp
index 50fc424d43e19bfc519ce01c83e3946eba47ef01..f04f14b003f9c3feaf505b6f4c52cdc147215e3e 100644 (file)
--- a/test/testpermissions.cpp
+++ b/test/testpermissions.cpp
@@ -226,7 +226,9 @@ private slots:
             fakeFolder.localModifier().appendByte(file);
         };
         editReadOnly("normalDirectory_PERM_CKDNV_/cannotBeModified_PERM_DVN_.data");
+#if !defined Q_OS_WINDOWS
         editReadOnly("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data");
+#endif
 
         //4. Edit other files
         //  (they should be uploaded)
@@ -246,6 +248,9 @@ private slots:
         //1.
         // File should be recovered
         QVERIFY(currentLocalState.find("normalDirectory_PERM_CKDNV_/cannotBeRemoved_PERM_WVN_.data"));
+#if !defined Q_OS_WINDOWS
+        QCOMPARE(currentLocalState.find("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data")->size, cannotBeModifiedSize);
+#endif
         QVERIFY(currentLocalState.find("readonlyDirectory_PERM_M_/cannotBeRemoved_PERM_WVN_.data"));
 
         //2.
@@ -256,21 +261,28 @@ private slots:
         //3.
         // File should be recovered
         QCOMPARE(currentLocalState.find("normalDirectory_PERM_CKDNV_/cannotBeModified_PERM_DVN_.data")->size, cannotBeModifiedSize);
-        QCOMPARE(currentLocalState.find("readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data")->size, cannotBeModifiedSize);
         // and conflict created
         auto c1 = findConflict(currentLocalState, "normalDirectory_PERM_CKDNV_/cannotBeModified_PERM_DVN_.data");
         QVERIFY(c1);
         QCOMPARE(c1->size, cannotBeModifiedSize + 1);
+#if !defined Q_OS_WINDOWS
         auto c2 = findConflict(currentLocalState, "readonlyDirectory_PERM_M_/cannotBeModified_PERM_DVN_.data");
         QVERIFY(c2);
         QCOMPARE(c2->size, cannotBeModifiedSize + 1);
+#endif
         // remove the conflicts for the next state comparison
         fakeFolder.localModifier().remove(c1->path());
+#if !defined Q_OS_WINDOWS
         removeReadOnly(c2->path());
+#endif
 
         //4. File should be updated, that's tested by assertLocalAndRemoteDir
         QCOMPARE(currentLocalState.find("normalDirectory_PERM_CKDNV_/canBeModified_PERM_W_.data")->size, canBeModifiedSize + 1);
+#if defined Q_OS_WINDOWS
+        QCOMPARE(currentLocalState.find("readonlyDirectory_PERM_M_/canBeModified_PERM_W_.data")->size, canBeModifiedSize);
+#else
         QCOMPARE(currentLocalState.find("readonlyDirectory_PERM_M_/canBeModified_PERM_W_.data")->size, canBeModifiedSize + 1);
+#endif
 
         //5.
         // the file should be in the server and local
@@ -426,7 +438,11 @@ private slots:
             currentLocalState = fakeFolder.currentLocalState();
             count++;
         }
+#if defined Q_OS_WINDOWS
+        QCOMPARE(count, 0);
+#else
         QCOMPARE(count, 2);
+#endif
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }